Skip to content

feat(@typegpu/gl): Implicit pointer definitions in GLSL generator - #2819

Merged
iwoplaza merged 1 commit into
feat/make-triangle-work-with-gl-fallbackfrom
feat/glsl-implicit-pointer-definitions
Aug 11, 2026
Merged

feat(@typegpu/gl): Implicit pointer definitions in GLSL generator#2819
iwoplaza merged 1 commit into
feat/make-triangle-work-with-gl-fallbackfrom
feat/glsl-implicit-pointer-definitions

Conversation

@iwoplaza

@iwoplaza iwoplaza commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Closes #2679

Copilot AI lite review requested due to automatic review settings August 9, 2026 20:35
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

pkg.pr.new

packages
Ready to be installed by your favorite package manager ⬇️

https://pkg.pr.new/software-mansion/TypeGPU/eslint-plugin-typegpu@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/tgpu-gen@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/tinyest-for-wgsl@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/typegpu@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/cli@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/color@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/gl@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/noise@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/radiance-cascades@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/react@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/sdf@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/@typegpu/three@19f55241db084c73b130fd996dd98c90cb54d94f
https://pkg.pr.new/software-mansion/TypeGPU/unplugin-typegpu@19f55241db084c73b130fd996dd98c90cb54d94f

benchmark
view benchmark

commit
view commit

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Resolution Time Benchmark

---
config:
  themeVariables:
    xyChart:
      plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
  title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
  x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
  y-axis "time (ms)"
  line [1.01, 2.01, 4.36, 6.42, 8.17, 11.44, 23.20, 23.43]
  line [1.04, 1.99, 4.27, 6.56, 8.16, 11.54, 22.65, 24.02]
  line [0.99, 2.07, 4.19, 6.42, 7.55, 11.92, 23.42, 23.87]
Loading
---
config:
  themeVariables:
    xyChart:
      plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
  title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
  x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
  y-axis "time (ms)"
  line [0.28, 0.49, 0.68, 0.84, 1.12, 1.18, 1.40, 1.58]
  line [0.32, 0.54, 0.75, 0.84, 1.16, 1.26, 1.53, 1.65]
  line [0.30, 0.53, 0.70, 0.91, 1.17, 1.26, 1.54, 1.61]
Loading
---
config:
  themeVariables:
    xyChart:
      plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
  title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
  x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
  y-axis "time (ms)"
  line [0.81, 2.12, 4.41, 6.63, 12.52, 26.65, 56.31, 111.15]
  line [0.84, 2.20, 4.11, 7.04, 12.76, 26.61, 57.55, 114.69]
  line [1.04, 2.22, 4.77, 6.60, 13.01, 27.16, 56.78, 118.90]
Loading

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Bundle size comparison (import * as ... in PR vs import * as ... in target):

🟢 Decreased (max -0.28%) ➖ Unchanged 🔴 Increased (max 2.79%) ❔ Unknown
6 130 187 1

import * as ... in PR vs import * as ... in target (did bundle size increase?):

Test tsdown
std_isBeingTranspiled.ts 15.71 kB ($${\color{red}+2.8\%}$$)
std_getTargetShaderLanguage.ts 15.77 kB ($${\color{red}+2.8\%}$$)
std_getShaderStage.ts 15.76 kB

import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):

Test tsdown
tgpu_init.ts 270.45 kB ($${\color{green}-3.3\%}$$)
tgpu_initFromDevice.ts 269.91 kB ($${\color{green}-3.5\%}$$)
tgpu_resolve.ts 170.71 kB ($${\color{green}-39.0\%}$$)
tgpu_resolveWithContext.ts 170.64 kB ($${\color{green}-39.0\%}$$)
tgpu_bindGroupLayout.ts 73.82 kB ($${\color{green}-73.6\%}$$)
tgpu_mutableAccessor.ts 68.55 kB ($${\color{green}-75.5\%}$$)
tgpu_accessor.ts 68.54 kB ($${\color{green}-75.5\%}$$)
tgpu_privateVar.ts 67.23 kB ($${\color{green}-76.0\%}$$)
tgpu_workgroupVar.ts 67.23 kB ($${\color{green}-76.0\%}$$)
tgpu_const.ts 66.65 kB ($${\color{green}-76.2\%}$$)
tgpu_lazy.ts 66.45 kB ($${\color{green}-76.2\%}$$)
tgpu_fragmentFn.ts 38.92 kB ($${\color{green}-86.1\%}$$)
tgpu_fn.ts 38.87 kB ($${\color{green}-86.1\%}$$)
tgpu_vertexFn.ts 38.74 kB ($${\color{green}-86.1\%}$$)
tgpu_computeFn.ts 38.44 kB ($${\color{green}-86.3\%}$$)
tgpu_vertexLayout.ts 27.57 kB ($${\color{green}-90.1\%}$$)
tgpu_comptime.ts 15.18 kB ($${\color{green}-94.6\%}$$)
tgpu_unroll.ts 1.75 kB ($${\color{green}-99.4\%}$$)
tgpu_slot.ts 1.70 kB ($${\color{green}-99.4\%}$$)

If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds GLSL-compatible handling for TypeGPU’s “implicit pointer” const-aliasing semantics, enabling the WebGL/GLSL fallback to preserve expected mutation behavior without native pointer support.

Changes:

  • Refactors WGSL const-alias handling into an overridable _aliasConstStatement hook.
  • Implements GLSL-specific aliasing rules: copy from immutable origins (e.g. uniforms), otherwise inline the aliased l-value expression and hoist runtime index expressions into temporaries to ensure single evaluation.
  • Adds a dedicated implicitPointer.test.ts suite covering immutable copies, mutable aliases, runtime index hoisting, and nested/alias-of-alias cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/typegpu/src/tgsl/wgslGenerator.ts Extracts implicit-pointer const-alias logic into _aliasConstStatement for reuse/override.
packages/typegpu-gl/src/glslGenerator.ts Overrides const-alias lowering to be pointer-free in GLSL via copy-or-alias + index-hoisting.
packages/typegpu-gl/tests/implicitPointer.test.ts Adds snapshot tests validating GLSL output for implicit pointer scenarios.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The substitution + index-hoisting design is sound and matches WGSL pointer semantics in the cases I traced, but the immutable branch copies aliased values via _emitVarDecl, which emits invalid GLSL ES 3.00 whenever the aliased value is array-typed (const a = u.arr;float a[4] = u.arr;, which GLSL ES 3.00 does not allow). See the inline comment for the fix.

Reviewed changes

  • GLSL alias const statements_aliasConstStatement override in glslGenerator.ts: copies aliases of immutable memory (uniform/readonly/handle) into locals, and turns mutable-memory aliases into compile-time substitutions of the aliased expression, hoisting non-constant index expressions into once-evaluated temp variables.
  • Index hoisting helper — new #hoistIndexAccesses walks member/index accesses and replaces runtime index expressions with @index_N references tied to freshly declared temps; comptime-constant indices are bound directly.
  • WGSL generator refactor — the implicit-pointer const handling in wgslGenerator.ts was extracted into a protected _aliasConstStatement hook with line-identical logic (makeUniqueIdentifier(rawId, 'block') preserved); WGSL generator/constant/struct tests still pass (139 tests).
  • New test suiteimplicitPointer.test.ts with six snapshot tests (immutable copy, mutable substitution, runtime-index hoisting, nested member/index chains, alias-of-an-alias, local-variable alias).

I verified the edge semantics directly: index expressions are snapshotted at the declaration (later reassignment of the index variable doesn't affect the alias, matching WGSL pointer-address semantics), and alias-of-alias chains resolve correctly through the stored expressions.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/typegpu-gl/src/glslGenerator.ts
Comment thread packages/typegpu-gl/tests/implicitPointer.test.ts
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from 12cf3a1 to 1e1ea6e Compare August 10, 2026 10:06
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from 1e1ea6e to 339c509 Compare August 10, 2026 10:17
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from 339c509 to 8715ec1 Compare August 11, 2026 09:16
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from 8715ec1 to abec4f8 Compare August 11, 2026 09:25
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from abec4f8 to f8f2a8b Compare August 11, 2026 13:35
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from f8f2a8b to e36ebd2 Compare August 11, 2026 14:43
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from e36ebd2 to 909d278 Compare August 11, 2026 14:50

@cieplypolar cieplypolar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool

Comment thread packages/typegpu-gl/src/glslGenerator.ts Outdated
Comment thread packages/typegpu-gl/src/glslGenerator.ts

@aleksanderkatan aleksanderkatan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!

@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch 2 times, most recently from c951b6e to 27fd9a1 Compare August 11, 2026 15:48

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one stale doc example to fix.

The rework since the last review is clean: dropping the @index_N placeholder scheme in favor of real idx-prefixed temps (unique via makeUniqueIdentifier('idx', 'block')), inlining comptime-constant indices guarded by !possibleSideEffects, and removing undecorateDataType from both branches. I verified the undecorateDataType removal is safe — @location-decorated fields still emit plain GLSL (vec3 p = u.pos;), consistent with the WGSL base _aliasConstStatement which also uses eq.dataType directly. The new multi-index test pins the single-evaluation semantics well.

Reviewed changes

  • Reworked #hoistIndexAccesses to substitute comptime-constant indices inline (now guarded by !index.possibleSideEffects) and to hoist runtime indices into once-evaluated idx temps, removing the @index_N placeholder scheme and the #hoistedIndexCount counter entirely.
  • Dropped undecorateDataType(...) from both the immutable-copy and mutable-alias branches of _aliasConstStatement so the alias type flows through undecorated, matching the WGSL base treatment.
  • Renamed hoisted temps itemidx across the generator and the snapshot suite, and added a hoists multiple index accessed test covering nested 2D indices (boids.$[index]![idx2]!) plus a user variable named idx2.
Technical details

The constant-index inlining branch now returns [NODE.indexAccess, target, node[2]] (the raw AST index) instead of binding a placeholder snippet; re-_expression of a comptime constant is deterministic, and the !possibleSideEffects guard makes the path strictly more conservative than before. Runtime indices register via defineVariable(name, snip(name, ...)), which is a self-referential identity that still lets makeUniqueIdentifier reserve the name — sound.

ℹ️ Nitpicks

  • Test description 'hoists multiple index accessed' — should read 'hoists multiple index accesses'.
  • The @example on #hoistIndexAccesses still shows the old item/item_1 naming, which no longer matches emitted code.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

*
* @example
* ```
* arr[foo()].prop[idx] => arr[item].prop[item_1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @example is stale after the itemidx rename: the emitted temps are now named via makeUniqueIdentifier('idx', 'block') (e.g. idx, idx_1), so the documented arr[item].prop[item_1] / 'int item = foo();' no longer matches actual output. The example's input variable idx also now collides conceptually with the generated temp prefix.

Suggested change
* arr[foo()].prop[idx] => arr[item].prop[item_1]
* arr[foo()].prop[j] => arr[idx].prop[idx_1]
* // out: ['int idx = foo();', 'int idx_1 = j;']

@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from 27fd9a1 to 0760dff Compare August 11, 2026 16:41
@iwoplaza
iwoplaza force-pushed the feat/glsl-implicit-pointer-definitions branch from 0760dff to 19f5524 Compare August 11, 2026 18:04
@iwoplaza
iwoplaza merged commit 46019cb into main Aug 11, 2026
11 of 16 checks passed
@iwoplaza
iwoplaza deleted the feat/glsl-implicit-pointer-definitions branch August 11, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(@typegpu/gl): Support implicit pointer definitions

5 participants